Fix: Improve error handling for Glances widgets #3657
Conversation
There was a problem hiding this comment.
I can take a real look at this soon but my first thought is that the logic to respect errors should just be moved inside Error, instead of all the other changes for removing it. and in info and gpu where you check for the error property presumably that should become the error for the Error component
|
Hi, I don't necessarily disagree, the only reason I wrote the code this way is because the Error component for the glances widget essentially just returns the div with the localised translation: So it was just to tidy the imports and dependencies up a bit. By the way, I'm on UK timezone so if I don't reply quickly I'm probably asleep! |
|
Sure, but separating them logically makes sense and that's also how the rest of the app does it. Also would significantly reduce the diff here. Let us know if youre OK making the change, I can also take care of it if thats better. |
|
Sure, let me make the changes and commit. |
|
I've now reintroduced the error component and import it within the container component. Following the implementation style in e.g. the Home Assistant widget the error handling is still done within the container component. Let me know if you want any other changes making. |
|
That wasnt exactly what I had in mind. Please take a look at the changes here. (also FYI https://eslint.org/docs/latest/rules/no-prototype-builtins ) and let me know if you have any thoughts or concerns |
|
Thanks for this, and thanks for the link that's good to know. As for the changes, it seems like there isn't a consistent implementation of the error handling (e.g. the Home Assistant ref I provided was similar to my own implementation but different to the one you have provided in the changes.) - either way the result will be the same and looks good to me. |
|
Point taken, I've gone back to a bit of a mix |
shamoon
changed the title
Proposed change
I have the Glances widgets deployed in my Homepage instance and noticed that when a host is unreachable the widgets do not handle the errors as the other widgets do. It results in an ugly error message which doesn't respect either the global or service-level
hideErrors. Here is an example from my deployment:I have rewritten parts of the components such that the
Errorcomponent is no longer needed, as it is now implemented within theContainercomponent. The service and error attributes are now passed into theContainercomponent, such that the service-levelhideErrorsflag can be checked. Additionally, the global settings are now imported into theContainercomponent so the globalhideErrorsflag can be checked. Within each component, if the service data containers an "error" key, the widget will now display an error message in line with other widgets. See the screenshot below:Additionally, see the below screenshot testing the service-level
hideErrorsflag set totruefor one widget:Type of change
Checklist: